appDisplay: Protect against source icon destruction

When dragging icons out of a folder dialog, there is a very peculiar
combination of steps that may break GNOME Shell:

 1. Open an app folder dialog
 2. Start dragging an icon to outside the grid
 3. Wait until the popdown animation starts
 4. Before it finishes, drop the icon
 5. See the warnings / crash

That's caused by the source icon being destroyed after the delayed
move timer starts, and before it finishes.

Protect against the source icon being destroyed before the delayed
move timeout triggers by connecting to the 'destroy' signal and
removing the timeout on the callback. Use a single field, called
'_delayedMoveData', to store all data related to delayed moves.

https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1447

diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index ce6754c5625d3fadc053e101fd3129564996ccb6..56b4a587144981996f538713a898c22d73c5d750 100644 (file)
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -199,8 +199,7 @@ var BaseAppView = GObject.registerClass({
         // Drag n' Drop
         this._lastOvershoot = -1;
         this._lastOvershootTimeoutId = 0;
-        this._delayedMoveId = 0;
-        this._targetDropPosition = null;
+        this._delayedMoveData = null;
 
         this._dragBeginId = 0;
         this._dragEndId = 0;
@@ -355,29 +354,40 @@ var BaseAppView = GObject.registerClass({
             return;
         }
 
-        if (!this._targetDropPosition ||
-            this._targetDropPosition.page !== page ||
-            this._targetDropPosition.position !== position) {
+        if (!this._delayedMoveData ||
+            this._delayedMoveData.page !== page ||
+            this._delayedMoveData.position !== position) {
             // Update the item with a small delay
             this._removeDelayedMove();
-            this._targetDropPosition = { page, position };
-
-            this._delayedMoveId = GLib.timeout_add(GLib.PRIORITY_DEFAULT,
-                DELAYED_MOVE_TIMEOUT, () => {
-                    this._moveItem(source, page, position);
-                    this._targetDropPosition = null;
-                    this._delayedMoveId = 0;
-                    return GLib.SOURCE_REMOVE;
-                });
+            this._delayedMoveData = {
+                page,
+                position,
+                source,
+                destroyId: source.connect('destroy', () => this._removeDelayedMove()),
+                timeoutId: GLib.timeout_add(GLib.PRIORITY_DEFAULT,
+                    DELAYED_MOVE_TIMEOUT, () => {
+                        this._moveItem(source, page, position);
+                        this._delayedMoveData.timeoutId = 0;
+                        this._removeDelayedMove();
+                        return GLib.SOURCE_REMOVE;
+                    }),
+            };
         }
     }
 
     _removeDelayedMove() {
-        if (this._delayedMoveId > 0) {
-            GLib.source_remove(this._delayedMoveId);
-            this._delayedMoveId = 0;
-        }
-        this._targetDropPosition = null;
+        if (!this._delayedMoveData)
+            return;
+
+        const { source, destroyId, timeoutId  } = this._delayedMoveData;
+
+        if (timeoutId > 0)
+            GLib.source_remove(timeoutId);
+
+        if (destroyId > 0)
+            source.disconnect(destroyId);
+
+        this._delayedMoveData = null;
     }
 
     _resetOvershoot() {
@@ -495,8 +505,8 @@ var BaseAppView = GObject.registerClass({
             return false;
 
         // Dropped before the icon was moved
-        if (this._targetDropPosition) {
-            const { page, position } = this._targetDropPosition;
+        if (this._delayedMoveData) {
+            const { page, position } = this._delayedMoveData;
 
             this._moveItem(source, page, position);
             this._removeDelayedMove();